Skip to content

Unpin and upgrade dependencies#263

Closed
harryzcy wants to merge 7 commits into
trivago:mainfrom
harryzcy:fix-cve-2023-45133
Closed

Unpin and upgrade dependencies#263
harryzcy wants to merge 7 commits into
trivago:mainfrom
harryzcy:fix-cve-2023-45133

Conversation

@harryzcy
Copy link
Copy Markdown
Contributor

@harryzcy harryzcy commented Oct 16, 2023

Upgrade @babel-traverse to non-vulnerable version 7.23.2. CVE-2023-45133
Unpin dependencies to permit future upgrades, without changing prettier-plugin-sort-imports.

Fix #262

@richardjelinek-fastest
Copy link
Copy Markdown

🙏🥺

expect(format(formatted, { parser: 'babel' })).toEqual(
`// first comment
// second comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point adding new lines ? Is this coming from an IDE formater ?

Copy link
Copy Markdown
Contributor Author

@harryzcy harryzcy Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are generated after @babel/generator >=7.19.x.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With 7.19.3 the tests pass, but 7.19.4 they fail. Probably due to this PR: babel/babel#14979

Copy link
Copy Markdown

@marklai1998 marklai1998 Oct 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Eldemarkki The PR tagged as 8.0.0-alpha.2 but the changelog saids released under 7.19.4.
Did they just messed up the release?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems its an unvoidable breaking change

@harryzcy
Copy link
Copy Markdown
Contributor Author

@ayusharma Could you help with reviewing this?

@marklai1998
Copy link
Copy Markdown

@byara

Copy link
Copy Markdown
Collaborator

@byara byara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution. Other than the new lines added to the snapshots, this looks good to me.
Can we figure out a way not to add those new lines? Otherwise, this is a breaking change

@harryzcy
Copy link
Copy Markdown
Contributor Author

Thank you for your contribution. Other than the new lines added to the snapshots, this looks good to me. Can we figure out a way not to add those new lines? Otherwise, this is a breaking change

I couldn't find any @babel/generator that controls it.

@harryzcy
Copy link
Copy Markdown
Contributor Author

@byara #266 only updates @babel/traverse and leaves other dependencies untouched.

That could be the non-breaking one. And this PR could lead to a breaking change. What do you think?

@byara
Copy link
Copy Markdown
Collaborator

byara commented Oct 23, 2023

The change for babel traverse is release in v4.2.1

@harryzcy harryzcy changed the title Fix CVE-2023-45133 of @babel/traverse Unpin and upgrade dependencies Oct 23, 2023
@byara
Copy link
Copy Markdown
Collaborator

byara commented Dec 5, 2024

This should be addressed in the v5. Please feel free to reopen in there is an issue.

@byara byara closed this Dec 5, 2024
@StavNoyAkur8
Copy link
Copy Markdown

This should be addressed in the v5. Please feel free to reopen in there is an issue.

v5 looks to have updated the dependencies but kept them pinned.

"dependencies": {
"@babel/generator": "7.26.2",
"@babel/parser": "7.26.2",
"@babel/traverse": "7.25.9",
"@babel/types": "7.26.0",
"javascript-natural-sort": "0.7.1",

That means next time there's a vulnerability found, we will again need to wait for an update here, or override the dependencies.

I can't reopen, since it's not my PR.

@byara byara reopened this Dec 5, 2024
@byara
Copy link
Copy Markdown
Collaborator

byara commented Dec 5, 2024

FYI @StavNoyAkur8 #322

@byara byara closed this Dec 5, 2024
@harryzcy harryzcy deleted the fix-cve-2023-45133 branch December 7, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CVE-2023-45133

8 participants